[libc++] Fix strict aliasing violation for `deque::const_iterator` #136067

frederick-vs-ja · 2025-04-17T01:13:07Z

When the allocators use fancy pointers, the internal map of deque stores fancy_ptr<T> objects, and the previous strategy accessed these objects via const fancy_ptr<const T> lvalues, which usually caused core language undefined behavior. Now const_iterator stores fancy_ptr<const fancy_ptr<T>> instead of fancy_ptr<const fancy_ptr<const T>>, and ABI break can happen when such two types have incompatible layouts.

This is necessary for reducing undefined behavior and constexpr support for deque in C++26, and I currently don't want to provide any way to opt-out of that behavior.

For some pathological combinations of allocators and fancy pointers, the rebinding trick doesn't work. These cases are rejected by static_assert.

The existing test coverage seems to be sufficient, except that the not-quite-fancy-pointer types in libcxx/test/libcxx/containers/sequences/deque/abi.compile.pass.cpp need to have operator* added (which is required for fancy pointers).

When the allocators use fancy pointers, the internal map of `deque` stores `fancy_ptr<T>` objects, and the previous strategy accessed these objects via `const fancy_ptr<const T>` lvalues, which usually caused core language undefined behavior. Now `const_iterator` stores `fancy_ptr<const fancy_ptr<T>>` instead of `fancy_ptr<const fancy_ptr<const T>>`, and ABI break can happen when such two types have incompatible layouts. This is necessary for reducing undefined behavior and `constexpr` support for `deque` in C++26, and I currently don't want to provide any way to opt-out of that behavior. Also removes redundant identity `static_cast` before and after type change. The existing test coverage seems to be sufficient.

llvmbot · 2025-04-17T01:13:45Z

@llvm/pr-subscribers-libcxx

Author: A. Jiang (frederick-vs-ja)

Changes

When the allocators use fancy pointers, the internal map of deque stores fancy_ptr<T> objects, and the previous strategy accessed these objects via const fancy_ptr<const T> lvalues, which usually caused core language undefined behavior. Now const_iterator stores fancy_ptr<const fancy_ptr<T>> instead of fancy_ptr<const fancy_ptr<const T>>, and ABI break can happen when such two types have incompatible layouts.

This is necessary for reducing undefined behavior and constexpr support for deque in C++26, and I currently don't want to provide any way to opt-out of that behavior.

Also removes redundant identity static_cast before and after type change.

The existing test coverage seems to be sufficient.

Full diff: https://github.com/llvm/llvm-project/pull/136067.diff

2 Files Affected:

(modified) libcxx/docs/ReleaseNotes/21.rst (+8)
(modified) libcxx/include/deque (+8-9)

diff --git a/libcxx/docs/ReleaseNotes/21.rst b/libcxx/docs/ReleaseNotes/21.rst
index 2091a713ea200..84d7d8cf7aab3 100644
--- a/libcxx/docs/ReleaseNotes/21.rst
+++ b/libcxx/docs/ReleaseNotes/21.rst
@@ -114,6 +114,14 @@ ABI Affecting Changes
   comparison between shared libraries, since all RTTI has the correct visibility now. There is no behaviour change on
   Clang.
 
+- The ``const_iterator`` member type of ``std::deque`` is now corrected to hold a (possibly fancy) pointer to the
+  (possibly fancy) pointer allocated in the internal map. E.g. when the allocators use fancy pointers, the internal map
+  stores ``fancy_ptr<T>`` objects, and the previous strategy accessed these objects via ``const fancy_ptr<const T>``
+  lvalues, which usually caused core language undefined behavior. Now ``const_iterator`` stores
+  ``fancy_ptr<const fancy_ptr<T>>`` instead of ``fancy_ptr<const fancy_ptr<const T>>``, and ABI break can happen when
+  such two types have incompatible layouts. This is necessary for reducing undefined behavior and ``constexpr`` support
+  for ``deque`` in C++26, so we do not provide any way to opt-out of that behavior.
+
 
 Build System Changes
 --------------------
diff --git a/libcxx/include/deque b/libcxx/include/deque
index e9846af5e5848..8df2a046e618d 100644
--- a/libcxx/include/deque
+++ b/libcxx/include/deque
@@ -504,13 +504,12 @@ public:
   using pointer       = typename __alloc_traits::pointer;
   using const_pointer = typename __alloc_traits::const_pointer;
 
-  using __pointer_allocator _LIBCPP_NODEBUG       = __rebind_alloc<__alloc_traits, pointer>;
-  using __const_pointer_allocator _LIBCPP_NODEBUG = __rebind_alloc<__alloc_traits, const_pointer>;
-  using __map _LIBCPP_NODEBUG                     = __split_buffer<pointer, __pointer_allocator>;
-  using __map_alloc_traits _LIBCPP_NODEBUG        = allocator_traits<__pointer_allocator>;
-  using __map_pointer _LIBCPP_NODEBUG             = typename __map_alloc_traits::pointer;
-  using __map_const_pointer _LIBCPP_NODEBUG       = typename allocator_traits<__const_pointer_allocator>::const_pointer;
-  using __map_const_iterator _LIBCPP_NODEBUG      = typename __map::const_iterator;
+  using __pointer_allocator _LIBCPP_NODEBUG  = __rebind_alloc<__alloc_traits, pointer>;
+  using __map _LIBCPP_NODEBUG                = __split_buffer<pointer, __pointer_allocator>;
+  using __map_alloc_traits _LIBCPP_NODEBUG   = allocator_traits<__pointer_allocator>;
+  using __map_pointer _LIBCPP_NODEBUG        = typename __map_alloc_traits::pointer;
+  using __map_const_pointer _LIBCPP_NODEBUG  = typename allocator_traits<__pointer_allocator>::const_pointer;
+  using __map_const_iterator _LIBCPP_NODEBUG = typename __map::const_iterator;
 
   using reference       = value_type&;
   using const_reference = const value_type&;
@@ -721,7 +720,7 @@ public:
   }
 
   _LIBCPP_HIDE_FROM_ABI const_iterator begin() const _NOEXCEPT {
-    __map_const_pointer __mp = static_cast<__map_const_pointer>(__map_.begin() + __start_ / __block_size);
+    __map_const_pointer __mp = __map_.begin() + __start_ / __block_size;
     return const_iterator(__mp, __map_.empty() ? 0 : *__mp + __start_ % __block_size);
   }
 
@@ -733,7 +732,7 @@ public:
 
   _LIBCPP_HIDE_FROM_ABI const_iterator end() const _NOEXCEPT {
     size_type __p            = size() + __start_;
-    __map_const_pointer __mp = static_cast<__map_const_pointer>(__map_.begin() + __p / __block_size);
+    __map_const_pointer __mp = __map_.begin() + __p / __block_size;
     return const_iterator(__mp, __map_.empty() ? 0 : *__mp + __p % __block_size);
   }

philnik777

I don't think we can do this as-is. AFAICT this changes the type of deque::const_iterator, which is way too big of an ABI break.

frederick-vs-ja · 2025-06-05T11:14:01Z

@philnik777 Now I'm attempting to keep the actual type of deque::const_iterator unchanged and change type of the stored (possibly fancy) pointer instead.
I'm afraid that the standard seems to allow users to write member pointer types that can't be restored by pointer_traits::rebind, and thus we're still unable to handle some pathological but "correct" (standard-conforming) user-defined allocators.

libcxx/test/libcxx/containers/sequences/deque/abi.compile.pass.cpp

philnik777 · 2025-08-12T12:17:43Z

libcxx/include/deque

-  using iterator = __deque_iterator<value_type, pointer, reference, __map_pointer, difference_type>;
-  using const_iterator =
+  using iterator       = __deque_iterator<value_type, pointer, reference, __map_pointer, difference_type>;
+  using const_iterator = // Use of __map_const_pointer is merely kept for API stability.


What does this comment mean? We don't need to keep internal names stable.

It's used for making the actual type name of const_iterator stable, as your previous comment #136067 (review) suggested.

Ah, so you mean ABI stability, not API stability.

philnik777 · 2025-08-12T12:19:03Z

libcxx/docs/ReleaseNotes/22.rst

+- The ``const_iterator`` member type of ``std::deque`` is now corrected to hold a (possibly fancy) pointer to the
+  (possibly fancy) pointer allocated in the internal map. E.g. when the allocators use fancy pointers, the internal map
+  stores ``fancy_ptr<T>`` objects, and the previous strategy accessed these objects via ``const fancy_ptr<const T>``
+  lvalues, which usually caused core language undefined behavior. Now ``const_iterator`` stores


Suggested change

lvalues, which usually caused core language undefined behavior. Now ``const_iterator`` stores

lvalues, which caused undefined behavior. Now ``const_iterator`` stores

Let's not talk about "core language" UB as-if we handled it differently from library UB. They're the same thing and can be exploited by the compiler the same.

Changed. But IIUC they're not quite same - core UB is required to cause constant evaluation failure, while library UB isn't.

Ok, yes, for constant evaluation there is that difference. AFAIK the main reason for that is that some UB in the library is impossible to detect though because some UB is significantly more high level, not that there is a fundamental difference.

frederick-vs-ja requested a review from a team as a code owner April 17, 2025 01:13

llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Apr 17, 2025

frederick-vs-ja mentioned this pull request Apr 17, 2025

[libc++] constexpr deque #129368

Draft

philnik777 requested changes Apr 17, 2025

View reviewed changes

frederick-vs-ja added 5 commits June 5, 2025 16:56

Switch to use internally rebinding

c624455

Merge branch 'main' into fix-deque-const_iterator

02f54f2

__conditional_t

1c1a4a9

Fix allocators and fancy pointers in deque/abi.compile.pass.cpp

bbaa690

Merge branch 'main' into fix-deque-const_iterator

0bb7fa0

frederick-vs-ja added 9 commits June 5, 2025 19:32

Make GCC happy

94dbfb4

Merge branch 'main' into fix-deque-const_iterator

7a4eccd

Make them meeting Cpp17NullablePointer

ba6007f

Fix typo

6aee963

_LIBCPP_HIDE_FROM_ABI

cedbf8a

Merge branch 'main' into fix-deque-const_iterator

4c35ff7

Merge branch 'main' into fix-deque-const_iterator

08ffe3f

Merge branch 'main' into fix-deque-const_iterator

ad7ff04

Retarget LLVM22

a976380

philnik777 reviewed Aug 12, 2025

View reviewed changes

Address review comments

56fcc31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[libc++] Fix strict aliasing violation for `deque::const_iterator` #136067

[libc++] Fix strict aliasing violation for `deque::const_iterator` #136067

Uh oh!

frederick-vs-ja commented Apr 17, 2025 •

edited

Loading

Uh oh!

llvmbot commented Apr 17, 2025

Uh oh!

philnik777 left a comment

Uh oh!

frederick-vs-ja commented Jun 5, 2025

Uh oh!

Uh oh!

Uh oh!

philnik777 Aug 12, 2025

Uh oh!

frederick-vs-ja Aug 12, 2025

Uh oh!

philnik777 Aug 12, 2025

Uh oh!

philnik777 Aug 12, 2025

Uh oh!

frederick-vs-ja Aug 13, 2025

Uh oh!

philnik777 Aug 13, 2025

Uh oh!

Uh oh!

	lvalues, which usually caused core language undefined behavior. Now ``const_iterator`` stores
	lvalues, which caused undefined behavior. Now ``const_iterator`` stores

[libc++] Fix strict aliasing violation for deque::const_iterator #136067

Are you sure you want to change the base?

[libc++] Fix strict aliasing violation for deque::const_iterator #136067

Uh oh!

Conversation

frederick-vs-ja commented Apr 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Apr 17, 2025

Uh oh!

philnik777 left a comment

Choose a reason for hiding this comment

Uh oh!

frederick-vs-ja commented Jun 5, 2025

Uh oh!

Uh oh!

Uh oh!

philnik777 Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

frederick-vs-ja Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

philnik777 Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

philnik777 Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

frederick-vs-ja Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

philnik777 Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

[libc++] Fix strict aliasing violation for `deque::const_iterator` #136067

[libc++] Fix strict aliasing violation for `deque::const_iterator` #136067

frederick-vs-ja commented Apr 17, 2025 •

edited

Loading